Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Vikunja service widget #4118

Merged
merged 21 commits into from
Oct 12, 2024
Merged

Feature: Vikunja service widget #4118

merged 21 commits into from
Oct 12, 2024

Conversation

vhsdream
Copy link
Contributor

Proposed change

Created a new service widget for the task management application Vikunja (feature request #1630).

Widget with default settings

image

Widget with enableTaskList option set to true

image

API calls output (2 API calls in total)

projects.json
tasks.json

Type of change

  • New service widget
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation only
  • Other (please explain)

Checklist:

  • If applicable, I have added corresponding documentation changes.
  • If applicable, I have reviewed the feature and / or service widget guidelines.
  • I have checked that all code style checks pass using pre-commit hooks and linting checks.
  • If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.

@vhsdream vhsdream marked this pull request as ready for review October 12, 2024 04:15
Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick notes for now. I’ll find some time to test

src/widgets/vikunja/component.jsx Outdated Show resolved Hide resolved
src/utils/config/service-helpers.js Outdated Show resolved Hide resolved
src/widgets/vikunja/component.jsx Outdated Show resolved Hide resolved
@shamoon
Copy link
Collaborator

shamoon commented Oct 12, 2024

Thanks for the changes. Overall pretty good, just made some changes as you'll see. Let me know if you have any questions / concerns of course

@shamoon shamoon enabled auto-merge (squash) October 12, 2024 05:51
auto-merge was automatically disabled October 12, 2024 12:53

Head branch was pushed to by a user without write access

@vhsdream
Copy link
Contributor Author

Hi, I like all the changes and optimizations, I'm learning a lot! However I had to make a few small adjustments in order for the widget to work: add > 0 back to the projectsData filter function - otherwise it returns everything in Projects, including Saved Filters, which are not technically Projects. I guess a way I could describe them as not being Projects, is if I delete a Saved Filter, Tasks that were in them are not deleted, whereas if I delete a Project, all Tasks under that Project are deleted too.

Then I re-appended .getTime() to a few places where you cleaned up the date calculations, since without it, my widget was not showing any tasks due in 7 days or less.

Without > 0 and .getTime():
image

With > 0 and .getTime():
image

If the dates were being correctly compared in your environment, perhaps there are differences between our environments (I'm using a Debian 12 LXC, which is what I run homepage-prod in) and I have an issue of some kind, like missing dependencies?

@vhsdream vhsdream requested a review from shamoon October 12, 2024 13:20
…ojects filter to remove Saved Filters"

This reverts commit 1877fc8.
@shamoon

This comment was marked as resolved.

@shamoon

This comment was marked as resolved.

@shamoon
Copy link
Collaborator

shamoon commented Oct 12, 2024

Oh lol it uses negative numbers for the saved filters project id! Ha

@shamoon
Copy link
Collaborator

shamoon commented Oct 12, 2024

The problem was the conversion in widget.js was still getting turned back into a string, take a look now.

Screenshot 2024-10-12 at 7 20 01 AM

@shamoon
Copy link
Collaborator

shamoon commented Oct 12, 2024

Actually we can do it like this (last commit), the default due date is always the same string and just add a flag for that to the tasks.

I also made it show tasks without a due date in the list, totally happy to remove that if you prefer, but kinda figure why not?

Screenshot 2024-10-12 at 7 22 27 AM

@shamoon shamoon enabled auto-merge (squash) October 12, 2024 14:24
@shamoon
Copy link
Collaborator

shamoon commented Oct 12, 2024

And just to close the loop, date comparison in JS doesnt need getTime (except == !==, see https://stackoverflow.com/a/493018

=)

Thanks for the widget, welcome to JS land

Copy link
Contributor Author

@vhsdream vhsdream left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! It works perfectly and I am happy with the change to list tasks without due date (now I see how we could have it not display the date for those).

Copy link
Contributor Author

@vhsdream vhsdream left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍

@shamoon shamoon merged commit 20048ff into gethomepage:dev Oct 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants